stream: add compose operator by rluvaton · Pull Request #44937 · nodejs/node

rluvaton · 2022-10-09T10:50:07Z

Start adding compose function #43664

I've started implementing from scratch and then realized that I don't need to, Using the compose function is enough.

The only problem I have is with the abort signal as the static compose doesn't seem to support it so I used the addAbortSignal but we won't pass the stream the signal as the rest of the operators

Cc @benjamingr

nodejs-github-bot · 2022-10-09T10:50:10Z

Review requested:

@nodejs/streams

lib/internal/streams/operators.js

rluvaton · 2022-10-09T13:13:54Z

Cc @ronag

#43664 (comment)

xtx1130

If you change compose to chain use, whether we should hide the old compose api?
documents is needed

rluvaton · 2022-10-18T08:38:08Z

Thanks for reviewing

If you change compose to chain use, whether we should hide the old compose api?

I don't think so, it's still a valuable api

documents is needed

I did not add a documentation yet cause @ronag said that this may not be merged, so I'll wait for merge approval and then I'll write the docs 😀

rluvaton · 2022-10-18T08:40:36Z

@ronag @benjamingr @jasnell should I keep developing it? I don't wanna waste my time in case we decide not to do it 😊

benjamingr · 2022-10-26T18:24:45Z

Hey sorry, just getting on top of things and will review.

benjamingr

I'm in favor of it, needs docs before it can land

rluvaton · 2022-10-26T19:22:34Z

Added validation, aside from the docs I think this PR is ready

It would be great if someone could help me with the docs

test/parallel/test-stream-compose.js

mcollina

lgtm

doc/api/stream.md

benjamingr · 2022-10-27T11:13:10Z

doc/api/stream.md

+
+
+
+See [`stream.compose`][] for more information.


Add an example please?

Added 👍🏻

benjamingr

Looks good sans a few changes - blocking so this doesn't land before then :)

rluvaton · 2022-10-27T22:55:00Z

What to do about the failing lint? it fails cause the line is too long, but the line is long because of the link which is not displayed...

nodejs-github-bot · 2022-10-28T05:36:33Z

CI: https://ci.nodejs.org/job/node-test-pull-request/47517/

nodejs-github-bot · 2022-10-30T08:49:18Z

CI: https://ci.nodejs.org/job/node-test-pull-request/47573/

nodejs-github-bot · 2022-10-31T11:41:05Z

CI: https://ci.nodejs.org/job/node-test-pull-request/47589/

nodejs-github-bot · 2022-10-31T13:33:03Z

Commit Queue failed

- Loading data for nodejs/node/pull/44937 ✔ Done loading data for nodejs/node/pull/44937 ----------------------------------- PR info ------------------------------------ Title stream: add compose operator (#44937) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch rluvaton:feat/add-compose-stream-operator -> nodejs:main Labels needs-ci Commits 7 - stream: add compose operator - stream: rename import - stream: add abort signal support - stream: add validation and refactor tests - stream: fix lint and add started adding docs - stream: added an example in the streams docs - stream: fix lint error Committers 1 - Raz Luvaton <16746759+rluvaton@users.noreply.github.com> PR-URL: https://github.com/nodejs/node/pull/44937 Reviewed-By: Matteo Collina Reviewed-By: Benjamin Gruenbaum Reviewed-By: Robert Nagy ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/44937 Reviewed-By: Matteo Collina Reviewed-By: Benjamin Gruenbaum Reviewed-By: Robert Nagy -------------------------------------------------------------------------------- ℹ This PR was created on Sun, 09 Oct 2022 10:50:07 GMT ✔ Approvals: 3 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/44937#pullrequestreview-1158053238 ✔ - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/44937#pullrequestreview-1161868722 ✔ - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/44937#pullrequestreview-1159123210 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2022-10-31T11:41:05Z: https://ci.nodejs.org/job/node-test-pull-request/47589/ - Querying data for job/node-test-pull-request/47589/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 44937 From https://github.com/nodejs/node * branch refs/pull/44937/merge -> FETCH_HEAD ✔ Fetched commits as ffa2e964e882..7e55077b63a6 -------------------------------------------------------------------------------- [main 91eaa5aa1f] stream: add compose operator Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun Oct 9 13:43:49 2022 +0300 2 files changed, 98 insertions(+) create mode 100644 test/parallel/test-stream-compose-operator.js [main 18e34bf99c] stream: rename import Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun Oct 9 13:46:14 2022 +0300 1 file changed, 2 insertions(+), 2 deletions(-) [main 12370446d3] stream: add abort signal support Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun Oct 9 16:04:37 2022 +0300 2 files changed, 55 insertions(+), 11 deletions(-) [main 52e55a1bb8] stream: add validation and refactor tests Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Wed Oct 26 22:02:02 2022 +0300 3 files changed, 30 insertions(+), 16 deletions(-) Auto-merging doc/api/stream.md [main b9d363ceb1] stream: fix lint and add started adding docs Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Wed Oct 26 22:21:10 2022 +0300 3 files changed, 24 insertions(+), 5 deletions(-) Auto-merging doc/api/stream.md [main 261e4c014b] stream: added an example in the streams docs Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Fri Oct 28 01:39:24 2022 +0300 1 file changed, 20 insertions(+), 2 deletions(-) Auto-merging doc/api/stream.md [main bcd6bd6930] stream: fix lint error Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sat Oct 29 23:09:44 2022 +0300 1 file changed, 4 insertions(+), 2 deletions(-) ✔ Patches applied There are 7 commits in the PR. Attempting autorebase. Rebasing (2/14)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
stream: add compose operator

PR-URL: #44937
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Robert Nagy ronagy@icloud.com

[detached HEAD 115742ad0f] stream: add compose operator
Author: Raz Luvaton 16746759+rluvaton@users.noreply.github.com
Date: Sun Oct 9 13:43:49 2022 +0300
2 files changed, 98 insertions(+)
create mode 100644 test/parallel/test-stream-compose-operator.js
Rebasing (3/14)
Rebasing (4/14)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
stream: rename import

PR-URL: #44937
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Robert Nagy ronagy@icloud.com

[detached HEAD ec8d16b956] stream: rename import
Author: Raz Luvaton 16746759+rluvaton@users.noreply.github.com
Date: Sun Oct 9 13:46:14 2022 +0300
1 file changed, 2 insertions(+), 2 deletions(-)
Rebasing (5/14)
Rebasing (6/14)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
stream: add abort signal support

PR-URL: #44937
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Robert Nagy ronagy@icloud.com

[detached HEAD 44d7c6d427] stream: add abort signal support
Author: Raz Luvaton 16746759+rluvaton@users.noreply.github.com
Date: Sun Oct 9 16:04:37 2022 +0300
2 files changed, 55 insertions(+), 11 deletions(-)
Rebasing (7/14)
Rebasing (8/14)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
stream: add validation and refactor tests

PR-URL: #44937
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Robert Nagy ronagy@icloud.com

[detached HEAD ee4bffce62] stream: add validation and refactor tests
Author: Raz Luvaton 16746759+rluvaton@users.noreply.github.com
Date: Wed Oct 26 22:02:02 2022 +0300
3 files changed, 30 insertions(+), 16 deletions(-)
Rebasing (9/14)
Rebasing (10/14)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
stream: fix lint and add started adding docs

PR-URL: #44937
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Robert Nagy ronagy@icloud.com

[detached HEAD 1f23e92eee] stream: fix lint and add started adding docs
Author: Raz Luvaton 16746759+rluvaton@users.noreply.github.com
Date: Wed Oct 26 22:21:10 2022 +0300
3 files changed, 24 insertions(+), 5 deletions(-)
Rebasing (11/14)
Rebasing (12/14)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
stream: added an example in the streams docs

PR-URL: #44937
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Robert Nagy ronagy@icloud.com

[detached HEAD e08fe74562] stream: added an example in the streams docs
Author: Raz Luvaton 16746759+rluvaton@users.noreply.github.com
Date: Fri Oct 28 01:39:24 2022 +0300
1 file changed, 20 insertions(+), 2 deletions(-)
Rebasing (13/14)
Rebasing (14/14)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
stream: fix lint error

PR-URL: #44937
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Robert Nagy ronagy@icloud.com

[detached HEAD 581d9ddecc] stream: fix lint error
Author: Raz Luvaton 16746759+rluvaton@users.noreply.github.com
Date: Sat Oct 29 23:09:44 2022 +0300
1 file changed, 4 insertions(+), 2 deletions(-)

Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/3361645437

nodejs-github-bot · 2022-10-31T13:57:04Z

Landed in ddb3ae7

PR-URL: #44937 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>

PR-URL: nodejs#44937 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>

PR-URL: #44937 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>

rluvaton added 2 commits October 9, 2022 13:43

stream: add compose operator

076bbe5

stream: rename import

da9255f

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Oct 9, 2022

stream: add abort signal support

273b0b6

rluvaton commented Oct 9, 2022

View reviewed changes

lib/internal/streams/operators.js Show resolved Hide resolved

rluvaton commented Oct 9, 2022

View reviewed changes

lib/internal/streams/operators.js Show resolved Hide resolved

xtx1130 reviewed Oct 18, 2022

View reviewed changes

benjamingr reviewed Oct 26, 2022

View reviewed changes

rluvaton added 2 commits October 26, 2022 22:02

stream: add validation and refactor tests

2e4898e

stream: fix lint and add started adding docs

fed2ac3

rluvaton commented Oct 26, 2022

View reviewed changes

test/parallel/test-stream-compose.js Show resolved Hide resolved

mcollina approved these changes Oct 27, 2022

View reviewed changes

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 27, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 27, 2022

benjamingr reviewed Oct 27, 2022

View reviewed changes

doc/api/stream.md Show resolved Hide resolved

benjamingr reviewed Oct 27, 2022

View reviewed changes

benjamingr requested changes Oct 27, 2022

View reviewed changes

stream: added an example in the streams docs

da244b9

rluvaton requested a review from benjamingr October 27, 2022 22:40

ronag approved these changes Oct 27, 2022

View reviewed changes

benjamingr approved these changes Oct 28, 2022

View reviewed changes

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 28, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 28, 2022

stream: fix lint error

7e55077

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 30, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 30, 2022

benjamingr approved these changes Oct 31, 2022

View reviewed changes

benjamingr added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 31, 2022

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 31, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 31, 2022

nodejs-github-bot merged commit ddb3ae7 into nodejs:main Oct 31, 2022

github-actions bot mentioned this pull request Nov 1, 2022

CI Reliability 2022-11-01 nodejs/reliability#417

Open

17 tasks

RafaelGSS pushed a commit that referenced this pull request Nov 1, 2022

stream: add compose operator

dfd75c1

PR-URL: #44937 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>

RafaelGSS mentioned this pull request Nov 1, 2022

v19.1.0 proposal #45269

Merged

github-actions bot mentioned this pull request Nov 2, 2022

CI Reliability 2022-11-02 nodejs/reliability#418

Open

15 tasks

rluvaton deleted the feat/add-compose-stream-operator branch November 4, 2022 09:29

RafaelGSS pushed a commit that referenced this pull request Nov 10, 2022

stream: add compose operator

ccb1c1e

PR-URL: #44937 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>

danielleadams pushed a commit that referenced this pull request Dec 30, 2022

stream: add compose operator

1c24d95

PR-URL: #44937 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>

danielleadams mentioned this pull request Dec 30, 2022

v18.13.0 release proposal #46025

Merged

danielleadams pushed a commit that referenced this pull request Dec 30, 2022

stream: add compose operator

3559ac1

PR-URL: #44937 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>

danielleadams pushed a commit that referenced this pull request Jan 3, 2023

stream: add compose operator

5f7d2b5

PR-URL: #44937 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>

Renegade334 mentioned this pull request Oct 1, 2024

stream: readable.compose fails to return a Duplex #55203

Closed

Uh oh!

Conversation

rluvaton commented Oct 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Oct 9, 2022

Uh oh!

Uh oh!

Uh oh!

rluvaton commented Oct 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xtx1130 left a comment

Choose a reason for hiding this comment

Uh oh!

rluvaton commented Oct 18, 2022

Uh oh!

rluvaton commented Oct 18, 2022

Uh oh!

benjamingr commented Oct 26, 2022

Uh oh!

benjamingr left a comment

Choose a reason for hiding this comment

Uh oh!

rluvaton commented Oct 26, 2022

Uh oh!

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

benjamingr Oct 27, 2022

Choose a reason for hiding this comment

Uh oh!

rluvaton Oct 27, 2022

Choose a reason for hiding this comment

Uh oh!

benjamingr left a comment

Choose a reason for hiding this comment

Uh oh!

rluvaton commented Oct 27, 2022

Uh oh!

nodejs-github-bot commented Oct 28, 2022

Uh oh!

nodejs-github-bot commented Oct 30, 2022

Uh oh!

nodejs-github-bot commented Oct 31, 2022

Uh oh!

nodejs-github-bot commented Oct 31, 2022

PR-URL: #44937 Reviewed-By: Matteo Collina matteo.collina@gmail.com Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com Reviewed-By: Robert Nagy ronagy@icloud.com

PR-URL: #44937 Reviewed-By: Matteo Collina matteo.collina@gmail.com Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com Reviewed-By: Robert Nagy ronagy@icloud.com

PR-URL: #44937 Reviewed-By: Matteo Collina matteo.collina@gmail.com Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com Reviewed-By: Robert Nagy ronagy@icloud.com

PR-URL: #44937 Reviewed-By: Matteo Collina matteo.collina@gmail.com Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com Reviewed-By: Robert Nagy ronagy@icloud.com

PR-URL: #44937 Reviewed-By: Matteo Collina matteo.collina@gmail.com Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com Reviewed-By: Robert Nagy ronagy@icloud.com

PR-URL: #44937 Reviewed-By: Matteo Collina matteo.collina@gmail.com Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com Reviewed-By: Robert Nagy ronagy@icloud.com

PR-URL: #44937 Reviewed-By: Matteo Collina matteo.collina@gmail.com Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com Reviewed-By: Robert Nagy ronagy@icloud.com

Successfully rebased and updated refs/heads/main.

Uh oh!

nodejs-github-bot commented Oct 31, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rluvaton commented Oct 9, 2022 •

edited

Loading

rluvaton commented Oct 9, 2022 •

edited

Loading

PR-URL: #44937
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Robert Nagy ronagy@icloud.com

PR-URL: #44937
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Robert Nagy ronagy@icloud.com

PR-URL: #44937
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Robert Nagy ronagy@icloud.com

PR-URL: #44937
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Robert Nagy ronagy@icloud.com

PR-URL: #44937
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Robert Nagy ronagy@icloud.com

PR-URL: #44937
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Robert Nagy ronagy@icloud.com

PR-URL: #44937
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Robert Nagy ronagy@icloud.com